add: Support LUKS encryption using IBM CEX secure keys on s390x by madhu-pillai · Pull Request #536 · coreos/butane

madhu-pillai · 2024-05-29T17:12:57Z

fcos/1.6.0-exp: Fix context for boot_device.layout errors

Use "layout" in the context path for errors related to the layout
entry in the boot_device configuration.
Add tests for those error cases.
Refactor mirror boot_device check for s390x.

Fixes: #484

vendor: Update ignition/v2 v2.19.0 & aws-sdk-go v1.53.5

Support LUKS encryption using IBM CEX secure keys on s390x

For fcos 1.6.0-exp & openshift 4.18.0-exp specs, expected to be based on
stable 3.5.0 spec.

See: coreos/ignition#1693
See: coreos/fedora-coreos-tracker#1708

docs/config-fcos-v1_6-exp.md

docs/config-flatcar-v1_2-exp.md

internal/doc/butane.yaml

madhu-pillai · 2024-07-11T04:55:14Z

Hi @travier ,
Can i've a review on this?

config/fcos/v1_6_exp/schema.go

madhu-pillai · 2024-07-14T10:41:19Z

Hi @travier ,
I've changed the schema part to Cex. Kindly review it.

madhu-pillai · 2024-07-14T10:48:36Z

$ butane % cat bin/arm64/openshift.bu 
variant: openshift
version: 4.17.0-experimental
metadata:
  name: MachineConfig
  labels:
    machineconfiguration.openshift.io/role: master1
boot_device:
  layout: s390x-eckd
  luks:
    device: /dev/dasda
    cex:
      enabled: true

 $ ./butane openshift.bu -o openshift.ign --pretty && cat openshift.ign
# Generated by Butane; do not edit
apiVersion: machineconfiguration.openshift.io/v1
kind: MachineConfig
metadata:
  labels:
    machineconfiguration.openshift.io/role: master1
  name: MachineConfig
spec:
  config:
    ignition:
      version: 3.5.0-experimental
    storage:
      filesystems:
        - device: /dev/mapper/root
          format: xfs
          label: root
          wipeFilesystem: true
      luks:
        - cex:
            enabled: true
          device: /dev/dasda2
          label: luks-root
          name: root
          wipeVolume: true

internal/doc/butane.yaml

prestist

It's coming along! a few nits;

Thank you for doing this.

config/common/errors.go

config/fcos/v1_6_exp/translate.go

config/flatcar/v1_2_exp/translate.go

internal/doc/butane.yaml

docs/examples.md

madhu-pillai · 2024-07-16T15:22:24Z

Hi @prestist ,
Could you please review the updated comment?.

prestist

Gennerally lgtm I have like one nit; otherwise I think its gtg.

config/common/errors.go

docs/examples.md

discussion: coreos/butane#536 (comment)

Docs rephrase in luks.cex.enabled description. coreos/butane#536 (comment)

madhu-pillai · 2024-10-22T06:56:47Z

[root@a3elp53 s390x]# ./butane boot.bu -o boot.ign --pretty
[root@a3elp53 s390x]# cat boot.bu
variant: fcos
version: 1.6.0-experimental
boot_device:
  layout: s390x-zfcp
  luks:
    device: /dev/sdb
    cex:
      enabled: true
[root@a3elp53 s390x]# cat boot.ign
{
  "ignition": {
    "version": "3.5.0-experimental"
  },
  "storage": {
    "filesystems": [
      {
        "device": "/dev/mapper/root",
        "format": "xfs",
        "label": "root",
        "wipeFilesystem": true
      }
    ],
    "luks": [
      {
        "cex": {
          "enabled": true
        },
        "device": "/dev/sdb4",
        "label": "luks-root",
        "name": "root",
        "wipeVolume": true
      }
    ]
  }
}

madhu-pillai · 2024-10-28T15:24:03Z

Hi, Is this pr ok to approve? or anything pending from my side?

prestist · 2024-11-11T19:23:01Z

@madhu-pillai , I am going to run through and review from my perspective, but will not be merging until I hear from @travier since he had a lot of feedback, and I want to ensure his concerns are resolved.

Note: the now latest version of ignition is 2.20.0 which has the functionality this sugar uses in stable, I would prefer to get this in before we update butane with that version and its new specs.

prestist

Please make sure that you have resolved, and fixed any review comments, there seems to be one which has gramar issues still here, and more importantly a question about structure here

madhu-pillai · 2024-11-12T09:49:33Z

Please make sure that you have resolved, and fixed any review comments, there seems to be one which has gramar issues still here, and more importantly a question about structure here

Hi @prestist ,
These reviews were already updated in the last commit. Sorry i missed to update the review message. Now done.

madhu-pillai · 2024-11-18T08:07:51Z

Hi @travier , Could you review the PR please?

docs/release-notes.md

vendor/modules.txt

docs/release-notes.md

internal/doc/butane.yaml

travier · 2024-11-20T15:56:33Z

OK, we need to split this in at least two commits:

vendor code update
addition of CEX support

travier · 2024-11-20T15:57:26Z

OK, we can not update to 2.20 until we do #558

travier · 2024-11-20T15:58:19Z

#550 (comment)

travier · 2024-11-20T16:10:16Z

Hold on, I'm doing the rebase / resplit.

travier · 2024-11-20T16:34:51Z

This should be good now. I'll do another pass tomorrow. Then we need to stabilize all of that.

prestist

With the latest changes, lgtm.

prestist · 2024-11-20T16:42:42Z

This should be good now. I'll do another pass tomorrow. Then we need to stabilize all of that.

@travier Makes sense, so to be clear, we land this pr then separately stabilize? (preferred) I don't want to stabilize in this pr.

travier · 2024-11-20T17:28:08Z

Yes, if I'm not mistaken, we should follow this order (#550 (comment)):

config/fcos/v1_6_exp/validate.go

- Use "layout" in the context path for errors related to the layout entry in the boot_device configuration. - Add tests for those error cases. - Refactor mirror boot_device check for s390x. Fixes: coreos#484

travier · 2024-11-21T16:30:28Z

Alright, I've added more tests and fixed the case where they failed so this should be ready now.

For fcos 1.6.0-exp & openshift 4.18.0-exp specs, expected to be based on stable 3.5.0 spec. See: coreos/ignition#1693 See: coreos/fedora-coreos-tracker#1708

prestist

Thank you for catching that/fixing it, and then adding tests. LGTM

madhu-pillai force-pushed the test_cex branch from 43fddf7 to 23b740a Compare May 29, 2024 17:19

madhu-pillai force-pushed the test_cex branch from 23b740a to 9d9959e Compare July 2, 2024 15:25

travier reviewed Jul 3, 2024

View reviewed changes

docs/config-fcos-v1_6-exp.md Outdated Show resolved Hide resolved

travier reviewed Jul 3, 2024

View reviewed changes

docs/config-flatcar-v1_2-exp.md Outdated Show resolved Hide resolved

travier reviewed Jul 3, 2024

View reviewed changes

internal/doc/butane.yaml Outdated Show resolved Hide resolved

prestist mentioned this pull request Jul 3, 2024

build(deps): bump github.com/coreos/ignition/v2 from 2.18.0 to 2.19.0 #539

Closed

madhu-pillai force-pushed the test_cex branch from 9d9959e to d26c2ad Compare July 9, 2024 09:56

travier reviewed Jul 12, 2024

View reviewed changes

config/fcos/v1_6_exp/schema.go Outdated Show resolved Hide resolved

madhu-pillai force-pushed the test_cex branch from d26c2ad to 3b2e899 Compare July 14, 2024 10:34

travier reviewed Jul 15, 2024

View reviewed changes

internal/doc/butane.yaml Show resolved Hide resolved

madhu-pillai force-pushed the test_cex branch from 3b2e899 to 166ca24 Compare July 15, 2024 16:53

prestist reviewed Jul 15, 2024

View reviewed changes

config/common/errors.go Outdated Show resolved Hide resolved

config/fcos/v1_6_exp/translate.go Show resolved Hide resolved

config/flatcar/v1_2_exp/translate.go Show resolved Hide resolved

internal/doc/butane.yaml Outdated Show resolved Hide resolved

docs/examples.md Show resolved Hide resolved

madhu-pillai force-pushed the test_cex branch from 166ca24 to 0551402 Compare July 16, 2024 06:18

prestist reviewed Aug 5, 2024

View reviewed changes

config/common/errors.go Outdated Show resolved Hide resolved

docs/examples.md Show resolved Hide resolved

madhu-pillai force-pushed the test_cex branch from 0551402 to 3ec0f5c Compare August 29, 2024 06:22

madhu-pillai added a commit to madhu-pillai/ignition that referenced this pull request Aug 30, 2024

config/doc/ignition.yaml: correction in description

570383f

discussion: coreos/butane#536 (comment)

madhu-pillai mentioned this pull request Aug 30, 2024

config/doc/ignition.yaml: correction in description coreos/ignition#1930

Merged

madhu-pillai added a commit to madhu-pillai/ignition that referenced this pull request Aug 30, 2024

config/doc/ignition.yaml: correction in description

0d91393

Docs rephrase in luks.cex.enabled description. coreos/butane#536 (comment)

madhu-pillai added a commit to madhu-pillai/ignition that referenced this pull request Sep 16, 2024

config/doc/ignition.yaml: correction in description

d11012b

Docs rephrase in luks.cex.enabled description. coreos/butane#536 (comment)

madhu-pillai force-pushed the test_cex branch from 3ec0f5c to f6f2964 Compare October 22, 2024 06:46

prestist requested changes Nov 11, 2024

View reviewed changes

prestist mentioned this pull request Nov 11, 2024

Release Butane 0.23.0 #550

Closed

40 tasks

madhu-pillai requested a review from prestist November 13, 2024 10:24

travier reviewed Nov 19, 2024

View reviewed changes

docs/release-notes.md Outdated Show resolved Hide resolved

travier reviewed Nov 19, 2024

View reviewed changes

vendor/modules.txt Show resolved Hide resolved

madhu-pillai force-pushed the test_cex branch from f6f2964 to b833c52 Compare November 20, 2024 14:25

travier reviewed Nov 20, 2024

View reviewed changes

docs/release-notes.md Outdated Show resolved Hide resolved

travier reviewed Nov 20, 2024

View reviewed changes

internal/doc/butane.yaml Outdated Show resolved Hide resolved

travier force-pushed the test_cex branch from 097f898 to 47cda85 Compare November 20, 2024 16:12

prestist approved these changes Nov 20, 2024

View reviewed changes

travier reviewed Nov 21, 2024

View reviewed changes

config/fcos/v1_6_exp/validate.go Outdated Show resolved Hide resolved

travier and others added 2 commits November 21, 2024 17:20

fcos/1.6.0-exp: Fix context for boot_device.layout errors

6531c7f

- Use "layout" in the context path for errors related to the layout entry in the boot_device configuration. - Add tests for those error cases. - Refactor mirror boot_device check for s390x. Fixes: coreos#484

vendor: Update ignition/v2 v2.19.0 & aws-sdk-go v1.53.5

50dff55

travier mentioned this pull request Nov 21, 2024

Configs with a boot_device.luks.device set but no method (tpm2, tang, etc.) set should fail validation #560

Closed

travier force-pushed the test_cex branch from 47cda85 to ccc3f27 Compare November 21, 2024 16:28

travier requested a review from prestist November 21, 2024 16:29

Support LUKS encryption using IBM CEX secure keys on s390x

5862ae0

For fcos 1.6.0-exp & openshift 4.18.0-exp specs, expected to be based on stable 3.5.0 spec. See: coreos/ignition#1693 See: coreos/fedora-coreos-tracker#1708

travier force-pushed the test_cex branch from ccc3f27 to 5862ae0 Compare November 21, 2024 16:38

travier approved these changes Nov 21, 2024

View reviewed changes

prestist approved these changes Nov 21, 2024

View reviewed changes

prestist merged commit da4b8a1 into coreos:main Nov 21, 2024

Comments

Conversation

madhu-pillai commented May 29, 2024 • edited by travier Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

madhu-pillai commented Jul 11, 2024

Uh oh!

Uh oh!

madhu-pillai commented Jul 14, 2024

Uh oh!

madhu-pillai commented Jul 14, 2024

Uh oh!

Uh oh!

prestist left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

madhu-pillai commented Jul 16, 2024

Uh oh!

prestist left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

madhu-pillai commented Oct 22, 2024

Uh oh!

madhu-pillai commented Oct 28, 2024

Uh oh!

prestist commented Nov 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

prestist left a comment

Choose a reason for hiding this comment

Uh oh!

madhu-pillai commented Nov 12, 2024

Uh oh!

madhu-pillai commented Nov 18, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

travier commented Nov 20, 2024

Uh oh!

travier commented Nov 20, 2024

Uh oh!

travier commented Nov 20, 2024

Uh oh!

travier commented Nov 20, 2024

Uh oh!

travier commented Nov 20, 2024

Uh oh!

prestist left a comment

Choose a reason for hiding this comment

Uh oh!

prestist commented Nov 20, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

travier commented Nov 20, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

travier commented Nov 21, 2024

Uh oh!

prestist left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

madhu-pillai commented May 29, 2024 •

edited by travier

Loading

prestist commented Nov 11, 2024 •

edited

Loading

prestist commented Nov 20, 2024 •

edited

Loading

travier commented Nov 20, 2024 •

edited

Loading